scripts/install_dependencies_apt.py (1)

20-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add type hints to the new installer helpers.

The new tuple-returning helpers are harder to follow without annotations.

As per coding guidelines, "Use type hints for function parameters and return values."

Also applies to: 40-68, 71-90, 100-127, 130-130

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/install_dependencies_apt.py` around lines 20 - 37, The new installer
helper functions, including _run and the related helper routines in
install_dependencies_apt.py, are missing type annotations for parameters and
return values, which makes the tuple-returning behavior harder to understand.
Add explicit type hints to each helper’s signature, including _run’s command
argument and boolean/string tuple return, and apply the same treatment to the
other newly added helper methods/functions referenced in the diff so their
inputs and outputs are clear and consistent with the coding guidelines.

Source: Coding guidelines

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

scripts/install_dependencies_apt.py (1)

9-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use structured logging for the new installer diagnostics.

These new status and failure paths still write directly to stdout, which makes Pi-side triage harder once the installer is run remotely or under a service manager. Please route the new messages through a module logger with a stable prefix/context instead of expanding the print() surface.

Suggested direction

+import logging
+
+LOGGER = logging.getLogger(__name__)
+
-print(f"Trying to install {apt_package} via apt...")
+LOGGER.info("[Dependency Install] Trying to install %s via apt", apt_package)

-print("DEPENDENCY INSTALLATION FAILURES - DETAILS")
+LOGGER.error("[Dependency Install] DEPENDENCY INSTALLATION FAILURES - DETAILS")

As per coding guidelines, **/*.py: "Implement comprehensive logging for remote debugging on Raspberry Pi" and "Use structured logging with context (e.g., [NHL Recent]) for logging messages".

Also applies to: 62-69, 89-99, 123-145, 197-227

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/install_dependencies_apt.py` around lines 9 - 18, The new installer
diagnostics in install_dependencies_apt.py are still using direct stdout prints;
update the affected status and failure paths to use the module logger instead.
Route messages through a logger with a stable contextual prefix such as the
existing installer context, and keep the logic in the installer helper functions
so remote/service-manager logs remain structured and searchable. Pay special
attention to the diagnostic flow around the command execution and error-summary
paths, replacing print-based output with logger calls while preserving the same
information.

Source: Coding guidelines

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

scripts/install_dependencies_apt.py (1)

9-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use structured logging for the new installer diagnostics.

These new status and failure paths still write directly to stdout, which makes Pi-side triage harder once the installer is run remotely or under a service manager. Please route the new messages through a module logger with a stable prefix/context instead of expanding the print() surface.

Suggested direction

+import logging
+
+LOGGER = logging.getLogger(__name__)
+
-print(f"Trying to install {apt_package} via apt...")
+LOGGER.info("[Dependency Install] Trying to install %s via apt", apt_package)

-print("DEPENDENCY INSTALLATION FAILURES - DETAILS")
+LOGGER.error("[Dependency Install] DEPENDENCY INSTALLATION FAILURES - DETAILS")

As per coding guidelines, **/*.py: "Implement comprehensive logging for remote debugging on Raspberry Pi" and "Use structured logging with context (e.g., [NHL Recent]) for logging messages".

Also applies to: 62-69, 89-99, 123-145, 197-227

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/install_dependencies_apt.py` around lines 9 - 18, The new installer
diagnostics in install_dependencies_apt.py are still using direct stdout prints;
update the affected status and failure paths to use the module logger instead.
Route messages through a logger with a stable contextual prefix such as the
existing installer context, and keep the logic in the installer helper functions
so remote/service-manager logs remain structured and searchable. Pay special
attention to the diagnostic flow around the command execution and error-summary
paths, replacing print-based output with logger calls while preserving the same
information.

Source: Coding guidelines

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

web_interface/blueprints/api_v3.py (2)

1684-1685: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Use the configured plugin directory in the fallback path.

When api_v3.plugin_manager is unavailable, this hardcodes plugin-repos/ and skips installs for deployments that use plugins/ or a custom plugin_system.plugins_directory. Reuse the same plugin-dir resolution pattern used elsewhere in this module instead of a fixed fallback.

Suggested fix

         elif action == 'install_plugin_requirements':
             active_pm = getattr(api_v3, 'plugin_manager', None)
-            plugins_dir = Path(active_pm.plugins_dir) if active_pm else PROJECT_ROOT / 'plugin-repos'
+            if active_pm:
+                plugins_dir = Path(active_pm.plugins_dir)
+            else:
+                plugin_system_config = (
+                    api_v3.config_manager.load_config().get('plugin_system', {})
+                    if api_v3.config_manager else {}
+                )
+                plugins_dir_name = plugin_system_config.get('plugins_directory', 'plugin-repos')
+                plugins_dir = (
+                    Path(plugins_dir_name)
+                    if os.path.isabs(plugins_dir_name)
+                    else PROJECT_ROOT / plugins_dir_name
+                )
             results = []

As per coding guidelines, "Handle different deployment contexts with environment awareness in code".

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

            active_pm = getattr(api_v3, 'plugin_manager', None)
            if active_pm:
                plugins_dir = Path(active_pm.plugins_dir)
            else:
                plugin_system_config = (
                    api_v3.config_manager.load_config().get('plugin_system', {})
                    if api_v3.config_manager else {}
                )
                plugins_dir_name = plugin_system_config.get('plugins_directory', 'plugin-repos')
                plugins_dir = (
                    Path(plugins_dir_name)
                    if os.path.isabs(plugins_dir_name)
                    else PROJECT_ROOT / plugins_dir_name
                )
            results = []

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@web_interface/blueprints/api_v3.py` around lines 1684 - 1685, The fallback
plugin path in api_v3.plugin_manager resolution is hardcoded to plugin-repos
instead of using the configured plugin directory. Update the plugin-dir lookup
in api_v3.py to reuse the same plugin-directory resolution pattern used
elsewhere in the module, so the fallback respects
plugin_system.plugins_directory and deployments that use plugins/. Keep the
logic anchored around active_pm and plugins_dir, but avoid a fixed PROJECT_ROOT
/ plugin-repos default.

Source: Coding guidelines

---

4627-4656: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Recurse into array items after converting JSON dicts to lists.

This fixes array-shaped dicts only at the current object level. If an array item is itself an object that contains schema-defined arrays, those nested arrays stay as numeric-keyed dicts and can still fail validation on JSON saves. Recurse through items.properties for converted/existing array elements too.

Suggested fix

         if 'application/json' in content_type and schema and 'properties' in schema:
             def _fix_json_arrays(cfg, props):
                 for k, ps in props.items():
                     if not isinstance(cfg, dict) or k not in cfg:
                         continue
                     pt = ps.get('type')
                     val = cfg[k]
                     if pt == 'array' and isinstance(val, dict):
                         keys = list(val.keys())
                         if keys and all(str(x).isdigit() for x in keys):
                             sorted_keys = sorted(keys, key=lambda x: int(str(x)))
                             items_schema = ps.get('items', {})
                             item_type = items_schema.get('type')
                             arr = [val[sk] for sk in sorted_keys]
                             if item_type in ('integer', 'number'):
                                 converted = []
                                 for v in arr:
                                     if isinstance(v, str):
                                         try:
                                             converted.append(int(v) if item_type == 'integer' else float(v))
                                         except (ValueError, TypeError):
                                             converted.append(v)
                                     else:
                                         converted.append(v)
                                 arr = converted
                             cfg[k] = arr
                         elif not keys:
                             cfg[k] = []
+                        val = cfg[k]
+                    if pt == 'array' and isinstance(val, list):
+                        items_schema = ps.get('items', {})
+                        if items_schema.get('type') == 'object' and 'properties' in items_schema:
+                            for item in val:
+                                if isinstance(item, dict):
+                                    _fix_json_arrays(item, items_schema['properties'])
                     elif pt == 'object' and 'properties' in ps and isinstance(val, dict):
                         _fix_json_arrays(val, ps['properties'])

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

            def _fix_json_arrays(cfg, props):
                for k, ps in props.items():
                    if not isinstance(cfg, dict) or k not in cfg:
                        continue
                    pt = ps.get('type')
                    val = cfg[k]
                    if pt == 'array' and isinstance(val, dict):
                        keys = list(val.keys())
                        if keys and all(str(x).isdigit() for x in keys):
                            sorted_keys = sorted(keys, key=lambda x: int(str(x)))
                            items_schema = ps.get('items', {})
                            item_type = items_schema.get('type')
                            arr = [val[sk] for sk in sorted_keys]
                            if item_type in ('integer', 'number'):
                                converted = []
                                for v in arr:
                                    if isinstance(v, str):
                                        try:
                                            converted.append(int(v) if item_type == 'integer' else float(v))
                                        except (ValueError, TypeError):
                                            converted.append(v)
                                    else:
                                        converted.append(v)
                                arr = converted
                            cfg[k] = arr
                        elif not keys:
                            cfg[k] = []
                        val = cfg[k]
                    if pt == 'array' and isinstance(val, list):
                        items_schema = ps.get('items', {})
                        if items_schema.get('type') == 'object' and 'properties' in items_schema:
                            for item in val:
                                if isinstance(item, dict):
                                    _fix_json_arrays(item, items_schema['properties'])
                    elif pt == 'object' and 'properties' in ps and isinstance(val, dict):
                        _fix_json_arrays(val, ps['properties'])
            _fix_json_arrays(plugin_config, schema['properties'])

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@web_interface/blueprints/api_v3.py` around lines 4627 - 4656, The
_fix_json_arrays helper in api_v3.py only normalizes array-shaped dicts at the
current object level, so nested arrays inside array items can remain
numeric-keyed dicts and break validation. Update _fix_json_arrays to recurse
into converted array elements when the schema’s items type is object (using
items.properties), and also handle existing list values by walking each element
so nested schemas are normalized too.

web_interface/templates/v3/partials/tools.html (1)

227-243: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Handle non-JSON error bodies before calling r.json().

Both fetch paths still assume error responses are JSON. If Flask, nginx, or a proxy returns an HTML/plain-text 500 page, the UI falls back to a raw SyntaxError instead of the intended friendly error message.

Suggested fix

-        fetch('/api/v3/system/action', {
+        fetch('/api/v3/system/action', {
             method: 'POST',
             headers: {'Content-Type': 'application/json'},
             body: JSON.stringify({action})
         })
-        .then(r => r.json())
+        .then(async r => {
+            const text = await r.text();
+            let data = {};
+            try { data = text ? JSON.parse(text) : {}; } catch {}
+            if (!r.ok) {
+                throw new Error(data.message || `Request failed (HTTP ${r.status})`);
+            }
+            return data;
+        })
@@
-        fetch('/api/v3/system/git-info')
-            .then(r => {
-                if (!r.ok) return r.json().then(d => Promise.reject(d.message || `HTTP ${r.status}`));
-                return r.json();
-            })
+        fetch('/api/v3/system/git-info')
+            .then(async r => {
+                const text = await r.text();
+                let d = {};
+                try { d = text ? JSON.parse(text) : {}; } catch {}
+                if (!r.ok) {
+                    throw new Error(d.message || `HTTP ${r.status}`);
+                }
+                return d;
+            })

Also applies to: 266-269

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@web_interface/templates/v3/partials/tools.html` around lines 227 - 243, The
fetch flow in the action handler currently calls r.json() unconditionally, so
HTML/plain-text error responses from /api/v3/system/action can surface as
SyntaxError instead of the intended friendly message. Update the fetch path in
the tools template to inspect the response before parsing, only parse JSON when
appropriate, and otherwise fall back to a safe text-based error message while
still passing a useful message into showResult. Apply the same handling to the
other matching fetch block referenced in the comment so both paths behave
consistently.